Change Symbol to VariableReferenceExpression in PlanNode#12606
Change Symbol to VariableReferenceExpression in PlanNode#12606rongrong merged 55 commits intoprestodb:masterfrom
Conversation
rongrong
force-pushed
the
symbol-to-variable
branch
4 times, most recently
from
da62e2a to
d8e7dd7
Compare
hellium01
left a comment
hellium01
left a comment
There was a problem hiding this comment.
It is so great that some one finally are trying to tackle this problem :) No more typeProviders!!!
There was a problem hiding this comment.
This should be VariableReference as well?
There was a problem hiding this comment.
All Symbol in all PlanNode should be variables. They can't all happen in one diff.
There was a problem hiding this comment.
We don't need outputSymbols here. We can convert to List<Symbol> from List<VariableReferenceExpression> very easily.
There was a problem hiding this comment.
I tried that first. It means a lot of unnecessary code change. I think adding variable separate from symbol then remove symbol is cleaner.
There was a problem hiding this comment.
Given we need to do this conversion very frequently, add a utility function in symbolAllocator to get VariableReferenceExpression?
context.getSymbolAllocator().newSymbol(...);
context.getSymbolAllocator().getVariableReference(symbol);
There was a problem hiding this comment.
It doesn't really matter. However you write it now, eventually these will be changed again. None of these will be here once the refactor is done.
There was a problem hiding this comment.
We can just replace all internal Symbol to Variable? We can do a conversion whenever we can getOutput.
There was a problem hiding this comment.
If you can get that work feel free to do so.
There was a problem hiding this comment.
No need to have duplicated makerSymbol and markerVariable in constructor.
There was a problem hiding this comment.
distinctSymbols/hashSymbol should be VariableReference?
There was a problem hiding this comment.
One thing at a time. Otherwise each diff will be huge.
rongrong
force-pushed
the
symbol-to-variable
branch
3 times, most recently
from
b1e1b27 to
5b97f9f
Compare
|
Not ready for review, only for entertainment. ^_^ |
rongrong
force-pushed
the
symbol-to-variable
branch
14 times, most recently
from
32fbc88 to
e99e7fc
Compare
rongrong
force-pushed
the
symbol-to-variable
branch
6 times, most recently
from
264493d to
191d74e
Compare
rongrong
force-pushed
the
symbol-to-variable
branch
from
070a623 to
5e1b38d
Compare
5 participants
Step 1: Add
outputVariablesto allPlanNodewhile keepingoutputSymbols. No logic change.Step 2: Start using
outputVariablesinstead ofoutputSymbolsin planning / optimization.Step 3: Remove
outputSymbols.This will take a while. I might or might not get it done eventually... -_-